Skip to content

feat(scopes): replace Hub with new scopes#2000

Open
Litarnus wants to merge 8 commits into5.xfrom
new-scopes
Open

feat(scopes): replace Hub with new scopes#2000
Litarnus wants to merge 8 commits into5.xfrom
new-scopes

Conversation

@Litarnus
Copy link
Contributor

This PR replaces the Hub based approach to the new scope based approach. The main difference is that instead of having one Hub with multiple Scope layers, data is now divided into different scopes, namely GlobalScope, IsolationScope and CurrentScope. Further details are documented here: https://develop.sentry.dev/sdk/telemetry/scopes/

# Conflicts:
#	src/Logs/LogsAggregator.php
#	src/Metrics/MetricsAggregator.php
#	src/SentrySdk.php
#	src/Tracing/Span.php
#	src/functions.php
#	tests/FunctionsTest.php
#	tests/Logs/LogsAggregatorTest.php
#	tests/Monolog/LogsHandlerTest.php
#	tests/State/HubTest.php
@Litarnus Litarnus marked this pull request as ready for review February 16, 2026 08:18
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.


if ($scope->propagationContext !== null && $scope->getType() !== ScopeType::current()) {
$this->propagationContext = $scope->propagationContext;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mergeFrom doesn't clone propagationContext unlike __clone does

Low Severity

mergeFrom assigns propagationContext by direct reference instead of cloning it, unlike __clone which deep-clones both user and propagationContext. This creates shared mutable state between the merged scope and the source (isolation) scope. Since PropagationContext has mutable setters and a lazy-initializing toBaggage() method, any mutation through the merged scope would unexpectedly affect the isolation scope's propagation context. The user field in mergeFrom is correctly cloned, making this inconsistency easy to overlook.

Additional Locations (1)

Fix in Cursor Fix in Web

if ($user->getEmail() !== null && !isset($scopeAttributes['user.email'])) {
$scopeAttributes['user.email'] = $user->getEmail();
}
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent user/scope-attribute precedence between logs and metrics

Medium Severity

In MetricsAggregator, user.id/user.email/user.name from getUser() are only added if the corresponding scope attribute does NOT already exist (!isset($scopeAttributes['user.id'])). In LogsAggregator, user data is set unconditionally after applyScopeAttributes, always overriding scope attributes with the same keys. This means scope attributes take precedence over user data for metrics but not for logs — an inconsistency introduced by this PR (the old code had no scope attribute checks for either).

Additional Locations (1)

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant